mlir/lib/Dialect/QCO/IR/Modifiers/PowOp.cpp (1)

68-76: ⚠️ Potential issue | 🔴 Critical

The rewrite set is broader than the body invariant it actually preserves.

verify() admits helper ops before the body unitary, but NegPowToInvPow and MoveCtrlOutside clone only the unitary, MergeNestedPow only retargets the moved op’s qubit operands, and the identity-producing branches replace pow with plain pass-through values. With body-local arith.* or captured values, that can dangle defs after replacement; with a nested parent modifier, it can leave the parent region with no unitary at all. Either tighten verify() back to the single-unitary form, or preserve/hoist the full prologue and rewrite enclosing modifiers in the same pattern.

Also applies to: 158-169, 183-199, 218-221, 244-259, 284-289, 482-499, 596-655

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mlir/lib/Dialect/QCO/IR/Modifiers/PowOp.cpp` around lines 68 - 76, The
rewrite patterns (NegPowToInvPow, MoveCtrlOutside, MergeNestedPow, and functions
like tryReplaceWithNamedPhaseGate operating on PowOp) assume a single-unitary
body but verify() currently allows helper ops before the unitary; this leads to
dangling defs or removal of the unitary while leaving prologue helpers behind.
Fix by either tightening verify() on PowOp to require exactly one unitary op in
the body (no prologue helpers), or update each rewrite (NegPowToInvPow,
MoveCtrlOutside, MergeNestedPow and tryReplaceWithNamedPhaseGate) to
preserve/hoist the full prologue: when cloning or retargeting the unitary, also
clone or move any preceding helper ops and update their uses, ensure
replacements replace the entire region (including helpers) or remap all
results/operands so no defs are left dangling, and update operand/region
remapping logic to maintain the same external SSA edges for PowOp.

mlir/lib/Dialect/QC/IR/Modifiers/PowOp.cpp (1)

60-68: ⚠️ Potential issue | 🔴 Critical

These rewrites assume a stricter body invariant than verify() enforces.

verify() currently permits arbitrary non-unitary ops before the body unitary, but the structural rewrites only preserve getBodyUnitary(), and some folds erase PowOp outright. With body-local arith.* defs, that can dangle operands after replacement; with a nested parent modifier, it can leave the parent body with no unitary at all. Either tighten the body back to unitary + yield, or hoist/inline the full prologue and rewrite the enclosing modifier atomically.

Based on learnings, InvOp::verify intentionally restricts the body region to exactly two operations (one unitary + yield).

Also applies to: 149-158, 170-173, 188-190, 204-207, 230-235, 431-450, 498-517

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

mlir/include/mlir/Dialect/QC/IR/QCOps.td (1)

1016-1022: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify negative fractional-power semantics in the op description.

Line 1022 still documents r < 0 as inv @ pow(-r) @ g unconditionally, but that rewrite is only sound for integer exponents. For fractional negative powers, the canonical form must stay as pow(-r){U} because principal-branch semantics differ on unitaries with eigenvalue -1 (e.g. H, X, Y, Z, ECR, SWAP).

Based on learnings, NegPowToInvPow is only mathematically correct for integer exponents and negative fractional exponents must not be documented as equivalent to pow(-r){inv{U}}.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/include/mlir/Dialect/QC/IR/QCOps.td` around lines 1016 - 1022, Update
the QCOps.td description for the modifier operation to clarify negative-power
semantics: state that the rewrite "r < 0: inv @ pow(-r) @ g" is only valid for
integer exponents and must not be applied to fractional negative exponents; for
negative fractional r the canonical form remains pow(-r){U} because
principal-branch semantics can differ for unitaries with eigenvalue -1 (e.g. H,
X, Y, Z, ECR, SWAP); also reference the transformation rule name NegPowToInvPow
to indicate it is restricted to integer exponents.

mlir/lib/Dialect/QC/IR/Modifiers/PowOp.cpp (1)

166-203: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Clone the full body when rebuilding nested modifiers.

NegPowToInvPow, MergeNestedPow, and MoveCtrlOutside only clone getBodyUnitary(). For cases like pow(-2) { rx(0.123) }, the arith.constant for 0.123 lives in the old body, so the cloned gate in the new modifier still references a value from the region being erased. That produces invalid IR and reintroduces the dangling-value failure you already handled in FoldPowIntoGate.

Please inline/move the non-YieldOp body ops together with the unitary, or clone/remap the unitary’s operand-def chain into the new body before replacing op.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/lib/Dialect/QC/IR/Modifiers/PowOp.cpp` around lines 166 - 203,
NegPowToInvPow, MergeNestedPow and MoveCtrlOutside currently only clone
getBodyUnitary() into the newly created modifier body which leaves upstream ops
(e.g. constants) referenced from the old region and causes dangling-value IR;
fix each pattern (the matchAndRewrite implementations that call
rewriter.replaceOpWithNewOp and PowOp::create / InvOp::create) to also clone or
move the non-Yield ops that the unitary depends on into the new region (or
clone/remap the unitary’s operand-def chain) before replacing the original op so
all SSA operands in the new body point to values inside the new region (similar
to the work done in FoldPowIntoGate but applied here to preserve constants and
other defs).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)

552-553: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix inconsistent ctrl MLIR example syntax.

This example is missing the targets keyword (qc.ctrl(%q0) (%a0 = %q1)), while the updated API/docs elsewhere use targets(...). Please align it to prevent copy/paste confusion.

Suggested doc fix

- * qc.ctrl(%q0) (%a0 = %q1) {                                                \
+ * qc.ctrl(%q0) targets(%a0 = %q1) {                                         \

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

   * qc.ctrl(%q0) targets(%a0 = %q1) {                                         \
   *   qc.OP_NAME(%PARAM1, %PARAM2) %a0 : !qc.qubit                            \

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h` around lines 552 -
553, The example documentation for the `qc.ctrl` operation in the
QCProgramBuilder.h file is missing the `targets` keyword in the syntax. Update
the example that currently shows `qc.ctrl(%q0) (%a0 = %q1)` to include the
`targets` keyword so it reads `qc.ctrl(%q0) targets(%a0 = %q1)` to align with
the current API documentation and prevent confusion for users copying the
example.

mlir/include/mlir/Dialect/Utils/Utils.h (1)

251-257: 🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="$(fd -p 'Utils.h' mlir/include | head -n1)"
echo "Inspecting: $FILE"
nl -ba "$FILE" | sed -n '245,262p'

echo
echo "Searching for similar unchecked std::next patterns:"
rg -n --type=cpp 'std::next\([^,]+,\s*static_cast<std::ptrdiff_t>\(i\)\)' mlir/include/mlir/Dialect

Repository: munich-quantum-toolkit/core

Length of output: 264

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the correct file
echo "=== Finding Utils.h in mlir/include/mlir/Dialect/Utils ==="
find mlir/include/mlir/Dialect/Utils -name "Utils.h" 2>/dev/null || echo "File not found with find"

echo
echo "=== Listing files in mlir/include/mlir/Dialect/Utils ==="
ls -la mlir/include/mlir/Dialect/Utils/ 2>/dev/null || echo "Directory not found"

echo
echo "=== Using git ls-files to find Utils.h ==="
git ls-files | grep -E "mlir/include.*Utils\.h$" | head -20

Repository: munich-quantum-toolkit/core

Length of output: 762

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Once we find the correct file, read lines 245-262
FILE="mlir/include/mlir/Dialect/Utils/Utils.h"
if [ -f "$FILE" ]; then
  echo "=== Content of $FILE (lines 245-262) ==="
  cat -n "$FILE" | sed -n '245,262p'
  
  echo
  echo "=== Full context around getBodyUnitary function ==="
  cat -n "$FILE" | sed -n '240,270p'
else
  echo "File $FILE not found"
fi

Repository: munich-quantum-toolkit/core

Length of output: 2538

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for std::next usage with iterator patterns
echo "=== Searching for std::next patterns in MLIR ==="
rg -n 'std::next' mlir/include/mlir/Dialect/Utils/ --type=cpp --type=hpp 2>/dev/null || echo "No matches found"

echo
echo "=== Checking for llvm::make_filter_range usage ==="
rg -n 'make_filter_range' mlir/include/ --type=cpp --type=hpp -A 3 -B 1 | head -50

Repository: munich-quantum-toolkit/core

Length of output: 311

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="mlir/include/mlir/Dialect/Utils/Utils.h"

echo "=== Checking header guard ==="
head -5 "$FILE"

echo
echo "=== Checking for `#pragma` once ==="
grep -n "pragma once" "$FILE" || echo "No `#pragma` once found"

echo
echo "=== Verifying surrounding code context ==="
cat -n "$FILE" | sed -n '1,20p'

Repository: munich-quantum-toolkit/core

Length of output: 1095

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for similar std::next patterns that might have the same issue
echo "=== Searching for std::next with distance in entire mlir/include ==="
rg -n 'std::next\s*\(' mlir/include --type cpp -A 1 -B 1 | head -80

echo
echo "=== Checking LLVM FilterRange in LLVM headers ==="
find mlir -name "*.h" -o -name "*.hpp" | xargs grep -l "make_filter_range" 2>/dev/null | head -5

Repository: munich-quantum-toolkit/core

Length of output: 679

---

🌐 Web query:

std::next iterator undefined behavior C++ standard forward iterator bounds checking

💡 Result:

In C++, std::next is defined in terms of std::advance [1][2]. The C++ standard does not require std::next to perform bounds checking [3][4]. Consequently, if you use std::next to advance an iterator beyond the valid range (e.g., past the end of a container), it results in undefined behavior [3][4]. The standard library's iterator operations assume that the caller ensures the resulting iterator remains within the bounds of a valid range (specifically, that every iterator between the start and the destination, inclusive, is non-singular and valid) [5][4]. Because there is no internal bounds checking, std::next will simply perform the underlying increment operations, leading to undefined behavior if those operations violate iterator requirements [3][4]. While the C++ standard does not provide bounds checking for std::next, some specific implementations or compilation modes may provide optional "checked" iterators that trigger runtime assertions or errors in debug builds [6][7]. However, these are not guaranteed by the standard and typically do not exist in release builds to maintain performance [7]. If you need to safely advance an iterator while respecting container bounds, you should either: 1. Check the distance manually (e.g., using std::distance or container size) before calling std::next [3]. 2. Use C++20's std::ranges::next, which provides overloads that accept a sentinel (such as end) to safely bound the advancement [8][9]. This allows you to advance an iterator without exceeding the specified limit [8][10].

Citations:

• 1: https://cppreference.dev/w/cpp/iterator/next
• 2: https://dev.cppreference.com/cpp/iterator/next
• 3: https://stackoverflow.com/questions/38305559/stdnext-with-n-stddistanceit-c-end
• 4: https://stackoverflow.com/questions/5916239/stdadvance-behavior-when-advancing-beyond-end-of-container
• 5: https://eel.is/c++draft/iterator.requirements.general
• 6: https://learn.microsoft.com/en-us/cpp/standard-library/checked-iterators?view=msvc-170
• 7: https://stackoverflow.com/questions/8864429/bound-check-for-iterator-in-release-mode-c
• 8: https://cppreference.dev/w/cpp/iterator/ranges/next
• 9: https://eel.is/c++draft/iterator.primitives
• 10: https://en.cppreference.com/cpp/iterator/ranges/next

---

Avoid undefined iterator advancement in getBodyUnitary.

At line 254, std::next(unitaries.begin(), i) can advance past end() before the bounds check executes, resulting in undefined behavior. Per the C++ standard, std::next performs no bounds checking—advancing an iterator beyond its valid range is undefined behavior regardless of subsequent condition checks.

🔧 Proposed fix

 template <typename UnitaryInterface>
 [[nodiscard]] UnitaryInterface getBodyUnitary(Block& block, size_t i) {
   auto unitaries = llvm::make_filter_range(
       block, [](Operation& op) { return isa<UnitaryInterface>(op); });
-  auto it = std::next(unitaries.begin(), static_cast<std::ptrdiff_t>(i));
+  auto it = unitaries.begin();
+  for (size_t n = 0; n < i && it != unitaries.end(); ++n) {
+    ++it;
+  }
   if (it == unitaries.end()) {
     llvm::reportFatalUsageError("Unitary index out of bounds");
   }
   return cast<UnitaryInterface>(*it);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/include/mlir/Dialect/Utils/Utils.h` around lines 251 - 257, The function
getBodyUnitary calls std::next to advance an iterator by i positions before
checking bounds, which results in undefined behavior since std::next performs no
bounds validation. To fix this, manually iterate through the filtered unitaries
range while counting elements, checking if the count reaches i and if iteration
completes without reaching the target index. Return the unitary operation when
the count matches i, or call llvm::reportFatalUsageError if iteration completes
without finding the i-th unitary.

mlir/lib/Conversion/QCOToQC/QCOToQC.cpp (1)

127-138: 🩺 Stability & Availability | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, verify the file exists and read the function at the specified lines
if [ -f "mlir/lib/Conversion/QCOToQC/QCOToQC.cpp" ]; then
  echo "=== File found ==="
  wc -l mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
  echo ""
  echo "=== Function inlineRegion (lines 127-138) ==="
  sed -n '127,145p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
else
  echo "File not found"
fi

Repository: munich-quantum-toolkit/core

Length of output: 1082

---

🏁 Script executed:

#!/bin/bash
# Search for the moveRegion function mentioned in the diff
echo "=== Searching for moveRegion ==="
fd -t f "\.cpp$|\.h$" mlir/ | xargs rg "moveRegion" -B 2 -A 5 | head -50

Repository: munich-quantum-toolkit/core

Length of output: 3926

---

🏁 Script executed:

#!/bin/bash
# Find the other call sites mentioned (821-823, 867-871, 912-919)
echo "=== Checking call sites in the file ==="
sed -n '821,823p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
sed -n '867,871p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
sed -n '912,919p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1082

---

🏁 Script executed:

#!/bin/bash
# Search for all calls to inlineRegion in the file
echo "=== All calls to inlineRegion ==="
rg "inlineRegion\s*\(" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -B 2 -A 2

Repository: munich-quantum-toolkit/core

Length of output: 1373

---

🏁 Script executed:

#!/bin/bash
# Search for moveRegion in QCOToQC.cpp specifically
echo "=== Searching for moveRegion in QCOToQC.cpp ==="
rg "moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo "Exit code: $?"
echo ""

# Check the full context around inlineRegion calls to understand type conversion
echo "=== Checking context around line 821 ==="
sed -n '810,830p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""

# Check class context - see if this is a pattern rewriter
echo "=== Searching for class context and pattern definition ==="
sed -n '1,50p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 2527

---

🏁 Script executed:

#!/bin/bash
# Look for type conversion or adaptor usage in the region operations
echo "=== Searching for adaptor usage around inlineRegion calls ==="
rg -B 5 "inlineRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | head -80

Repository: munich-quantum-toolkit/core

Length of output: 1985

---

🏁 Script executed:

#!/bin/bash
# Check the QCOToJeff moveRegion implementation to understand the pattern
echo "=== Full moveRegion implementation in QCOToJeff.cpp ==="
rg "static LogicalResult moveRegion" mlir/lib/Conversion/QCOToJeff/QCOToJeff.cpp -A 30

Repository: munich-quantum-toolkit/core

Length of output: 1421

---

🏁 Script executed:

#!/bin/bash
# Check if there's any type conversion or adaptor in QCOToQC patterns
echo "=== Searching for TypeConverter usage in QCOToQC.cpp ==="
rg "TypeConverter\|getTypeConverter" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 222

---

🏁 Script executed:

#!/bin/bash
# Find where moveRegion is called in QCOToQC.cpp
echo "=== Full context of moveRegion calls in QCOToQC.cpp ==="
rg "moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -B 10 -A 3

Repository: munich-quantum-toolkit/core

Length of output: 1306

---

🏁 Script executed:

#!/bin/bash
# Check if moveRegion is defined in QCOToQC.cpp
echo "=== Search for moveRegion definition ==="
rg "static.*moveRegion\|LogicalResult.*moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -A 15

Repository: munich-quantum-toolkit/core

Length of output: 206

---

🏁 Script executed:

#!/bin/bash
# Look at the complete ForOp pattern to understand type conversion concern
echo "=== Complete ForOp pattern class ==="
sed -n '809,850p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1554

---

🏁 Script executed:

#!/bin/bash
# Check if there's any type conversion happening with adaptor values
echo "=== Adaptor type context in patterns ==="
rg "struct.*Adaptor\|class.*Adaptor\|OpAdaptor" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -A 5 | head -50

Repository: munich-quantum-toolkit/core

Length of output: 206

---

🏁 Script executed:

#!/bin/bash
# Check the return type and error handling in current inlineRegion to see if it's supposed to fail
echo "=== Checking if void return is problematic ==="
sed -n '120,160p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1959

---

🏁 Script executed:

#!/bin/bash
# Find all function definitions in QCOToQC.cpp
echo "=== All function definitions ==="
ast-grep outline mlir/lib/Conversion/QCOToQC/QCOToQC.cpp --view expanded | grep -A 2 "moveRegion\|inlineRegion"

Repository: munich-quantum-toolkit/core

Length of output: 198

---

🏁 Script executed:

#!/bin/bash
# Search more broadly for moveRegion in the file
echo "=== All occurrences of moveRegion with context ==="
grep -n "moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 364

---

🏁 Script executed:

#!/bin/bash
# Check line counts around the moveRegion usages - are they before or after inlineRegion?
echo "=== Lines in QCOToQC.cpp ==="
wc -l mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== Lines with moveRegion ==="
grep -n "moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== Lines with inlineRegion ==="
grep -n "inlineRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 979

---

🏁 Script executed:

#!/bin/bash
# Check if moveRegion is a helper lambda or a separate function
echo "=== Context around first moveRegion call (around line 1001) ==="
sed -n '995,1005p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 840

---

🏁 Script executed:

#!/bin/bash
# Look for any function that might be moveRegion before the first call
echo "=== Search in the area before first moveRegion call ==="
sed -n '100,200p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | grep -A 5 -B 5 "moveRegion\|inlineRegion"

Repository: munich-quantum-toolkit/core

Length of output: 1097

---

🏁 Script executed:

#!/bin/bash
# Look at the moveRegion calls with full context
echo "=== moveRegion call at line 649 ==="
sed -n '640,660p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== moveRegion call at line 687 ==="
sed -n '678,698p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1537

---

🏁 Script executed:

#!/bin/bash
# Find the moveRegion function definition - it must be defined somewhere
echo "=== Search for moveRegion definition in entire file ==="
sed -n '1,700p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | grep -B 5 -A 30 "moveRegion"

Repository: munich-quantum-toolkit/core

Length of output: 1756

---

🏁 Script executed:

#!/bin/bash
# Check if it's imported from a header
echo "=== Check headers for moveRegion ==="
head -50 mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | grep -i "include"
echo ""
echo "=== Search for moveRegion in header files ==="
find mlir/lib/Conversion -name "*.h" -exec grep -l "moveRegion" {} \;

Repository: munich-quantum-toolkit/core

Length of output: 1282

---

🏁 Script executed:

#!/bin/bash
# Let's look at the full section from line 640-700 to see the complete pattern
echo "=== Complete pattern around moveRegion calls ==="
sed -n '630,700p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 2146

---

🏁 Script executed:

#!/bin/bash
# Search for moveRegion definition anywhere in the codebase
echo "=== Search for moveRegion definition across all files ==="
rg "^\s*auto moveRegion\s*=|^\s*static.*LogicalResult moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -A 20

Repository: munich-quantum-toolkit/core

Length of output: 223

---

🏁 Script executed:

#!/bin/bash
# Check if moveRegion might be a lambda defined in the pass structure
echo "=== Check the full pass structure ==="
sed -n '1040,1098p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 2742

---

🏁 Script executed:

#!/antml:bash
# Search backwards from first moveRegion call to find definition
echo "=== Search in lines 1-648 for moveRegion definition ==="
sed -n '1,648p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | tail -100 | grep -B 20 -A 5 "moveRegion\|^static"

Repository: munich-quantum-toolkit/core

Length of output: 221

---

🏁 Script executed:

#!/bin/bash
# Let's check the include file that defines the pass
echo "=== Check QCOToQC.h ==="
find mlir -name "QCOToQC.h" -exec cat {} \;

Repository: munich-quantum-toolkit/core

Length of output: 660

---

🏁 Script executed:

#!/bin/bash
# Search more carefully - moveRegion might be defined as a member function of the pass
echo "=== Full context of the pass class definition ==="
sed -n '950,1098p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | head -150

Repository: munich-quantum-toolkit/core

Length of output: 6008

---

🏁 Script executed:

#!/bin/bash
# Search the entire repository for moveRegion definition
echo "=== All moveRegion definitions in the repo ==="
rg "static.*LogicalResult.*moveRegion\|auto\s+moveRegion\s*=" --type cpp --type h -B 2 -A 15

Repository: munich-quantum-toolkit/core

Length of output: 212

---

🏁 Script executed:

#!/antml:bash
# Check around the ConvertQCOCtrlOp and ConvertQCOInvOp to understand what moveRegion they're calling
echo "=== Full ConvertQCOCtrlOp struct ==="
sed -n '628,666p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1303

---

🏁 Script executed:

#!/bin/bash
# Check if this conversion is actually compiling - moveRegion must be defined somewhere
echo "=== Check if file builds - look for compilation artifacts ==="
find . -name "CMakeLists.txt" -path "*/QCOToQC*" -exec cat {} \;

Repository: munich-quantum-toolkit/core

Length of output: 1922

---

🏁 Script executed:

#!/bin/bash
# Search all files in Conversion directory for moveRegion
echo "=== All references to moveRegion in Conversion directory ==="
rg "moveRegion" mlir/lib/Conversion/ --type cpp --type h | head -30

Repository: munich-quantum-toolkit/core

Length of output: 1595

---

🏁 Script executed:

#!/bin/bash
# This is key - moveRegion is called in QCOToQC.cpp but not defined. Let's verify this is a real issue
echo "=== Check if QCOToQC.cpp compiles or if moveRegion is missing ==="
rg "moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -B 5 -A 2 | head -60
echo ""
echo "=== Check for static moveRegion in QCOToQC.cpp ==="
rg "static.*moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo "Exit code: $?"
echo ""
echo "=== Check all helper functions defined in QCOToQC.cpp before the patterns ==="
sed -n '100,650p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp | grep -B 2 -A 10 "^static\|^auto"

Repository: munich-quantum-toolkit/core

Length of output: 2299

---

🏁 Script executed:

#!/bin/bash
# Check QCToQCO.cpp to see if it has the same issue
echo "=== Check QCToQCO.cpp for moveRegion definition ==="
rg "static.*moveRegion\|LogicalResult moveRegion" mlir/lib/Conversion/QCToQCO/QCToQCO.cpp -A 15

Repository: munich-quantum-toolkit/core

Length of output: 217

---

🏁 Script executed:

#!/bin/bash
# Let's check the ConversionUtils header to see if moveRegion is defined there
echo "=== Check ConversionUtils.h for moveRegion ==="
find mlir -name "ConversionUtils.h" -exec cat {} \; | grep -B 2 -A 15 "moveRegion"

Repository: munich-quantum-toolkit/core

Length of output: 851

---

🏁 Script executed:

#!/bin/bash
# Check if there's a common header that defines moveRegion
echo "=== Check all includes in QCOToQC.cpp that might have moveRegion ==="
head -40 mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== Look for moveRegion in those header files ==="
find mlir/Conversion -name "*.h" | xargs grep -l "moveRegion" 2>/dev/null

Repository: munich-quantum-toolkit/core

Length of output: 1589

---

🏁 Script executed:

#!/bin/bash
# Get the full ConversionUtils.h definition to understand moveRegion
echo "=== Full ConversionUtils.h content ==="
find mlir -name "ConversionUtils.h" -type f -exec cat {} \;

Repository: munich-quantum-toolkit/core

Length of output: 1910

---

🏁 Script executed:

#!/bin/bash
# Now verify the concern - check if SCF patterns using inlineRegion with adaptor values
echo "=== Check ForOp pattern with adaptor.getInitArgs() ==="
sed -n '809,835p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== Check WhileOp pattern with adaptor values ==="
sed -n '854,876p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== Check IfOp pattern with adaptor values ==="
sed -n '900,925p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 3098

---

🏁 Script executed:

#!/bin/bash
# Verify that CtrlOp and InvOp already use moveRegion successfully
echo "=== CtrlOp using moveRegion ==="
sed -n '641,664p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "=== InvOp using moveRegion ==="
sed -n '679,702p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Repository: munich-quantum-toolkit/core

Length of output: 1535

---

🏁 Script executed:

#!/bin/bash
# Verify that adaptor values are indeed type-converted
# Check how OpAdaptor works with type conversion in MLIR patterns
echo "=== Understanding the issue: adaptor provides converted types ==="
echo "In SCF patterns:"
echo "- op.getInitArgs() returns QCO-typed values (original operation)"
echo "- adaptor.getInitArgs() returns QC-typed values (type-converted)"
echo ""
echo "When inlineRegion is called with adaptor values:"
echo "- Block args in inlined region still have QCO types"
echo "- Then they're replaced with QC-typed adaptor values"
echo "- This violates MLIR's same-type RAUW invariant"
echo ""
echo "=== Confirm the pattern is used correctly elsewhere ==="
rg "inlineRegion\|moveRegion" mlir/lib/Conversion/QCOToQC/QCOToQC.cpp -B 3 | grep -A 3 "inlineRegion\|moveRegion" | head -100

Repository: munich-quantum-toolkit/core

Length of output: 638

---

🏁 Script executed:

#!/bin/bash
# Check if there are any compile errors or test failures related to this
echo "=== Check test file to understand expected behavior ==="
find mlir -path "*/test*" -name "*qco_to_qc*" -type f | head -5

Repository: munich-quantum-toolkit/core

Length of output: 274

---

🏁 Script executed:

#!/bin/bash
# Verify return type consistency - inlineRegion is void but moveRegion returns LogicalResult
echo "=== Function signatures comparison ==="
echo "inlineRegion signature:"
sed -n '127,129p' mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
echo ""
echo "moveRegion signature (from ConversionUtils.h):"
echo "inline LogicalResult moveRegion(...)"
echo ""
echo "This means the proposed fix needs to:"
echo "1. Change inlineRegion to return LogicalResult"
echo "2. Update all callers to handle the return value"

Repository: munich-quantum-toolkit/core

Length of output: 657

---

Update SCF region inline patterns to convert block argument types before RAUW.

inlineRegion (lines 127-138) inlines regions with original block-argument types, then replaces those args using type-converted adaptor values. This violates MLIR's same-type RAUW invariant when adaptor values have different types (e.g., QC-typed) than the block arguments (e.g., QCO-typed).

The SCF conversion patterns (ForOp at 822, WhileOp at 868-870, IfOp at 913-918) pass type-converted adaptor values to inlineRegion, triggering this issue. However, the QCO control-flow patterns (CtrlOp, InvOp) correctly use moveRegion from ConversionUtils.h to convert block signatures during the inline operation, avoiding the invariant violation.

Change inlineRegion to use the type converter to convert block argument signatures before argument replacement, similar to the moveRegion pattern already in use. Update the function signature to return LogicalResult and modify all call sites to handle the result.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/lib/Conversion/QCOToQC/QCOToQC.cpp` around lines 127 - 138, The
inlineRegion function currently inlines regions and then attempts to replace
block arguments with type-converted adaptor values, violating MLIR's same-type
RAUW invariant when the adaptor values have different types than the original
block arguments. Modify inlineRegion to use the type converter to convert the
block argument types before performing the replaceAllUsesWith operation,
following the pattern used by moveRegion from ConversionUtils.h. Change the
function signature to return LogicalResult instead of void to properly
communicate conversion failures. Update all call sites of inlineRegion (in the
ForOp pattern around line 822, WhileOp pattern around lines 868-870, and IfOp
pattern around lines 913-918) to handle the returned LogicalResult.

mlir/lib/Dialect/QCO/IR/Modifiers/InvOp.cpp (3)

343-355: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve parent modifier validity when erasing empty inv.

Line 354 removes the InvOp even when it is the sole body unitary of another modifier, leaving the parent body with only qco.yield. Use an IdOp for the single-target nested case and bail for multi-target nested identity, matching the pow(0) handling.

🛡️ Suggested nested-modifier guard

-    rewriter.replaceOp(op, op.getOperands());
+    if (isa<CtrlOp, InvOp, PowOp>(op->getParentOp())) {
+      if (op.getNumTargets() != 1) {
+        return failure();
+      }
+      rewriter.replaceOpWithNewOp<IdOp>(op, op.getInputQubit(0));
+    } else {
+      rewriter.replaceOp(op, op.getOperands());
+    }
     return success();
   }
 };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/lib/Dialect/QCO/IR/Modifiers/InvOp.cpp` around lines 343 - 355, The
EraseEmptyInv struct's matchAndRewrite method removes InvOp operations without
checking if they are nested as the sole body unitary within another modifier,
which violates modifier body constraints. Before the current check on
op.getNumBodyUnitaries() in matchAndRewrite, add logic to detect if the InvOp is
nested inside a parent modifier body and handle it accordingly: for
single-target nested cases, replace with an IdOp instead of removing it; for
multi-target nested cases, return failure to prevent the rewrite. This approach
should mirror the handling used in the pow(0) pattern to maintain parent
modifier validity.

---

403-430: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Require a YieldOp before reading terminator operands.

Line 426 dereferences block.getTerminator() without proving the body is non-empty and yield-terminated. Malformed IR should get a verifier diagnostic instead of a null dereference or a non-yield “terminator” being accepted.

🛠️ Suggested verifier guard

-  auto* blockTerminator = block.getTerminator();
-  if (const auto numYieldOperands = blockTerminator->getNumOperands();
+  if (block.empty() || !isa<YieldOp>(block.back())) {
+    return emitOpError(
+        "last operation in body region must be a yield operation");
+  }
+  auto yieldOp = cast<YieldOp>(block.back());
+  if (const auto numYieldOperands = yieldOp->getNumOperands();
       numYieldOperands != numTargets) {
     return emitOpError("yield operation must yield ")
            << numTargets << " values, but found " << numYieldOperands;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/lib/Dialect/QCO/IR/Modifiers/InvOp.cpp` around lines 403 - 430, The
InvOp::verify() method dereferences block.getTerminator() without first
validating that the body is non-empty and that the terminator is specifically a
YieldOp. Before line 426 where getTerminator() is called, add a verification
check to ensure the terminator exists and is a YieldOp instance, returning an
appropriate emitOpError if either condition fails. This will prevent null
dereferences and reject malformed IR that contains non-yield terminators.

---

448-517: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Constrain matrix availability to the cases getUnitaryMatrix() implements.

Line 449 returns true for multi-target bodies with multiple known unitaries, but lines 514-515 return std::nullopt for those cases. Include the same zero/sole-unitary/single-target-composition conditions in the predicate.

🛠️ Minimal predicate alignment

 bool InvOp::hasCompileTimeKnownUnitaryMatrix() {
+  if (getNumBodyUnitaries() == 0) {
+    return true;
+  }
+  if (utils::getSoleBodyUnitary<UnitaryOpInterface>(*getBody())) {
+    return all_of(getBody()->getOps<UnitaryOpInterface>(),
+                  [](UnitaryOpInterface op) {
+                    return op.hasCompileTimeKnownUnitaryMatrix();
+                  });
+  }
+  if (getNumTargets() != 1) {
+    return false;
+  }
   return all_of(getBody()->getOps<UnitaryOpInterface>(),
                 [](UnitaryOpInterface op) {
                   return op.hasCompileTimeKnownUnitaryMatrix();
                 });
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mlir/lib/Dialect/QCO/IR/Modifiers/InvOp.cpp` around lines 448 - 517, The
hasCompileTimeKnownUnitaryMatrix() function returns true for cases where
getUnitaryMatrix() returns std::nullopt, causing inconsistency. Modify the
hasCompileTimeKnownUnitaryMatrix() predicate to include the same conditional
checks that getUnitaryMatrix() implements: it should verify that
getNumBodyUnitaries() returns zero, or that there is a sole body unitary
available, or that getNumTargets() equals one (for the composition case). This
ensures the predicate only returns true when getUnitaryMatrix() will
successfully return a matrix rather than std::nullopt.